Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Synchronize porting guide with precice-v3 #298

Merged
merged 1 commit into from
Nov 13, 2023

Conversation

BenjaminRodenberg
Copy link
Member

@BenjaminRodenberg BenjaminRodenberg commented Nov 11, 2023

I just did a merge of master into precice-v3 and resolved the resulting merge conflict in the porting guide.

After resolving the merge conflict, I created this branch and did git checkout precice-v3 pages/docs/couple-your-code/couple-your-co…de-porting-v2-3.md. The reson for this PR is to be able to review my resolution of the merge conflict.

To avoid this in the future: Generally, we should always keep the porting guide on master and precice-v3 synchronized, because there is no reason to not directly publish changes in the porting guide to master. The purpose of precice-v3 is to develop the documentation that we cannot publish yet, because it would overwrite the v2 documentation that is still relevant for users.

There are two important rules:

  1. If you are only changing the porting guide, make sure to do this on master. Don't forget to merge master into precice-v3 (e.g. Porting guide hint for removed preallocation option #288 and Clarify some instructions in porting guide #287).
  2. If there are changes in the porting guide that result from PRs with documentation updates on precice-v3, you should make sure to synchronize the porting guide with master.

Relevant changes to review:

@davidscn #290
@carme-hp #287
@IshaanDesai 91efb0a
@MakisH 778ce08
@fsimonis c8740c3

@BenjaminRodenberg
Copy link
Member Author

BenjaminRodenberg commented Nov 11, 2023

@davidscn @carme-hp @MakisH @IshaanDesai @fsimonis: If you see something here that is wrong: Please directly fix it on the branch.

@BenjaminRodenberg BenjaminRodenberg added this to the Version 3.0.0 milestone Nov 11, 2023
Copy link
Member

@IshaanDesai IshaanDesai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good 👍 thanks for resolving the conflicts.

Copy link
Member

@MakisH MakisH left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All changes make sense. I checked all commits, everything seems to be there.

Comment on lines -32 to +36
- Replace `precice::constants::*` with `isActionRequired()` and `markActionFulfilled()` with their respective requirement clause: `requiresInitialData()`, `requiresReadingCheckpoint()` or `requiresWritingCheckpoint()`. If these requirements are checked, then they are promised to be acted on.
- Replace `isMeshConnectivityRequired` with `requiresMeshConnectivityFor`.
- Replace `isGradientDataRequired` with `requiresGradientDataFor`. Instead of the input argument dataID, pass the meshName and dataName.
- Replace `isActionRequired()` with their respective requirement clause: `requiresInitialData()`, `requiresReadingCheckpoint()` or `requiresWritingCheckpoint()`.
- Remove `precice::constants::*` and corresponding `#include` statements as they are no longer needed.
- Remove `markActionFullfiled()`. If `requiresInitialData()`, `requiresReadingCheckpoint()`, or `requiresWritingCheckpoint()` are called, then they are promised to be acted on. Therefore, `markActionFullfiled()` is no longer needed.
- Replace `isMeshConnectivityRequired` with `requiresMeshConnectivityFor`. Instead of the input argument `meshID`, pass the `meshName`.
- Replace `isGradientDataRequired` with `requiresGradientDataFor`. Instead of the input argument `dataID`, pass the `meshName` and `dataName`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like how this part is broken down to simpler, clearer steps. 👍
(whoever did that: good job! -- that's @IshaanDesai )

@MakisH MakisH merged commit 539d87b into master Nov 13, 2023
2 checks passed
@MakisH MakisH deleted the synchronize-porting-guide-precicev3 branch November 13, 2023 17:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants